🐛 Fixed 404 in collection index page if using data.slug #10718
Conversation
| @@ -28,6 +28,13 @@ const defaultQueryOptions = { | |||
| } | |||
| }; | |||
|
|
|||
| const defaultDataQueryOptions = { | |||
There was a problem hiding this comment.
@kirrg001 changed the config to be more explicit. Think it's pretty readable this way
There was a problem hiding this comment.
Looks okay as a hot fix, but IMO the includes shouldn't be here at all. They should be part of the target api version config (long-term).
| @@ -69,6 +69,8 @@ function getPostSchema(metaData, data) { | |||
| var description = metaData.excerpt ? escapeExpression(metaData.excerpt) : null, | |||
| schema; | |||
|
|
|||
| const context = data.page ? 'page' : 'post'; | |||
There was a problem hiding this comment.
Does this work with v0.1? v0.1 had no differentiation for posts & pages.
getPostSchema is not only used for the "data" property. It's also used if you serve a post or a page.
There was a problem hiding this comment.
It would default to post in that case, but will double check 👍
There was a problem hiding this comment.
Verified and works as expected. Merging these changes
| @@ -69,6 +69,8 @@ function getPostSchema(metaData, data) { | |||
| var description = metaData.excerpt ? escapeExpression(metaData.excerpt) : null, | |||
| schema; | |||
|
|
|||
| const context = data.page ? 'page' : 'post'; | |||
There was a problem hiding this comment.
Was thinking of adding a parameter context which would be for the caller - getSchema to specify, but didn't go that route as it's not really guaranteed what the properties would be present if e.g. amp condition kicks in. Additionally, the logic below depends on the post/page property being present and the 'context' check - _.includes(context, 'post') is not directly guaranteeing that I think.
These ifs for post or page are another sign of issues with meta layer 🤷♂️
naz
force-pushed
the
fix-broken-collection-index
branch
2 times, most recently
from
e334b99 to
1a04af0
Compare
closes TryGhost#10542 - Fixed error that was causing collection index to not be rendered: relations connected to alliased resource were not fetched
refs TryGhost#10082 - When specifying an existing page as an allias for collection, e.g: `data: page.it-is-a-page` it was failing to generate metadata
naz
force-pushed
the
fix-broken-collection-index
branch
from
1a04af0 to
63fa899
Compare
closes #10542
@kirrg001 seems like a pretty straight forward fix but maybe you could think of some of the edge cases that could've been broken. One thing that I was puzzled about for a while when testing is that alliased resource in
data: {resource.slug}e.g.:data: post.knowwasn't accessible anymore and redirected me to the collection page, but later found the answer in the docs:The source route of the data will be redirected to the new custom route.😬